-
Notifications
You must be signed in to change notification settings - Fork 243
Dapper: micro-optimizations #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dapper: micro-optimizations #31
Conversation
…ut connection opening if single-op; use AsList to use pre-existing buffered list when available
Hi @mgravell, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
(nods to DNFBOT in a cheery way) |
|
||
result.AddRange(await db.QueryAsync<Fortune>("SELECT [Id], [Message] FROM [Fortune]")); | ||
// note: don't need to open connection if only doing one thing; let dapper do it | ||
result = (await db.QueryAsync<Fortune>("SELECT [Id], [Message] FROM [Fortune]")).AsList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming Dapper doesn't do any pre-sizing of the list, yes? I ask because it's a requirement of the test that it doesn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean "to match the expected data": then no, dapper doesn't do that, because it is consuming a raw IDataReader
(DbDataReader
on core-clr) API, and doesn't know the number of rows until it reaches the end. In this setup, it is essentially:
List<T> buffer = new List<T>();
while (await reader.ReadAsync(cancel).ConfigureAwait(false)) {
buffer.Add(func(reader));
}
while (await reader.NextResultAsync(cancel).ConfigureAwait(false)) { }
return buffer;
(where func
is a delegate that processes each row into the target data type)
Reference source is here; note that command.Buffered
is true
by default (most people screw up with things like concurrent commands, if handed back an open reader - so this defaults to the safest option of consuming the data before handing it back to the caller)
I ask because it's a requirement of the test that it doesn't.
Do you have a link to the test requirements? I would be happy to check them all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @benaadams ; I have reviewed the requirements; dapper does nothing that would violate Test 4 rule 5, if that is the concern.
Dapper: micro-optimizations
(from: dapper team) use QueryFirstOrDefault (in 1.50-beta4); let dapper worry about connection opening if single-op; use AsList to use pre-existing buffered list when available